Skip to content

Conversation

@quaff
Copy link
Contributor

@quaff quaff commented Oct 10, 2024

Alternative to #8971


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-18629

@quaff quaff marked this pull request as draft October 10, 2024 09:05
@quaff quaff force-pushed the 18629 branch 2 times, most recently from 647bd5e to 6517870 Compare October 11, 2024 01:25
@quaff quaff marked this pull request as ready for review October 11, 2024 01:47
@quaff
Copy link
Contributor Author

quaff commented Oct 14, 2024

Ready to review @beikov

Comment on lines 660 to 667
if ( resultEntityClass != null ) {
boolean exists = false;
for ( ResultBuilder existing : resultSetMapping.getResultBuilders() ) {
if ( resultEntityClass == existing.getJavaType() ) {
exists = true;
break;
}
}
if ( !exists ) {
addEntity( (Class<R>) resultEntityClass, LockMode.READ );
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more thinking of creating a ResultSetMapping on demand instead of altering the existing one to avoid interference with user configured mappings.

Suggested change
if ( resultEntityClass != null ) {
boolean exists = false;
for ( ResultBuilder existing : resultSetMapping.getResultBuilders() ) {
if ( resultEntityClass == existing.getJavaType() ) {
exists = true;
break;
}
}
if ( !exists ) {
addEntity( (Class<R>) resultEntityClass, LockMode.READ );
}
}
final ResultSetMapping mapping;
if ( resultJavaType != null && resultSetMapping.isDynamic() && resultSetMapping.getNumberOfResultBuilders() == 0 ) {
mapping = ResultSetMapping.resolveResultSetMapping( originalSqlString, true, getSessionFactory() );
mapping.addResultBuilder(
getSessionFactory().getMappingMetamodel().isEntityClass( resultJavaType )
? Builders.entityCalculated( StringHelper.unqualify( entityType.getName() ), entityType.getName(), LockMode.READ, getSessionFactory() )
: Builders.resultClassBuilder( resultClass, getSessionFactory() )
);
}
else {
mapping = resultSetMapping;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amended as you suggested, please note resultEntityClass always is entity class or null.

@sebersole
Copy link
Member

sebersole commented Nov 21, 2024

Could you rebase this on current main and repush please? I'm having trouble doing it myself locally.

You may have failures in EntityReturnClassTests after as I added those recently for https://hibernate.atlassian.net/browse/HHH-18864 which is the same as https://hibernate.atlassian.net/browse/HHH-18629 I think.

@beikov
Copy link
Member

beikov commented Nov 21, 2024

Superseded by #9301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants